-
Notifications
You must be signed in to change notification settings - Fork 215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integration Candidate 20191030 #281
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rename to "vxworks" and "posix", respectively, to match the actual implementation names. The build scripts rely on the name being the same and will fail if different. This also updates the default build to be vxworks rather than vxworks6, if not specified on the command line.
Add numerous unit test cases to improve the code coverage ratios on the vxworks and shared implementation layers. Prior to this change set, the coverage ratio was: lines......: 90.4% (2549 of 2820 lines) functions..: 95.9% (306 of 319 functions) After this change set, the coverage ratio is: lines......: 99.9% (2846 of 2849 lines) functions..: 100.0% (330 of 330 functions) Note these stats include some of the UT code itself, and this is what added 11 functions. No functions were added to FSW code. Note, one test condition will fail until the fix for related bug #269 is merged. This also fixes the posix coverage test so it builds and runs, but coverage is still not implemented here.
The timer code for VxWorks was fixed in bug #271 and the coverage code test needs a corresponding update to cover the code change. This is kept as a separate update commit as neither changeset is merged to master yet.
The test for failure of taskSpawn should be for the value of ERROR, not 0, per the VxWorks API documentation. Found when implementing the unit test improvements in #230. This issue is generally only reproducible in UT where taskSpawn can be made to fail.
On VxWorks, no minimum stack size is implemented, and the implementation will create tasks with an extremely small stack if "0" is passed in. This causes the worker tasks to overrun the stack on Vxworks, triggering undefined/inconsistent behavior.
Cast the address value so it may be printed as an integer.
In VxWorks, the sigwait routine was not handling the actual time of the first interval, which basically meant that the start_time was elapsed twice before the first callback was generated. This exposed a few other small but significant details regarding how the intervals were dealt with across all operating systems. For VxWorks and RTEMS, where the timer interval is rounded up to an integer number of ticks, compute the actual interval time and write a debug message in case it is different than the intended value. This makes it more obvious to the user that some application/bsp changes might be necessary to get accurate timing. For all operating systems, the "reset" flag needs to be sampled AFTER the sigwait returns, not before it. This flag gets set when the OS_TimeBaseSet() API is called. Removes an unused POSIX delay routine that was intended to support use of clock_nanosleep() in place of the timer signal. This mode was never usable because OS_TimerCreate always allocates a signal and fails if it cannot do so. This code caused problems/incompatibilities for the sigwait model which IS used, so it needed to be removed.
If the tick_time from the wait routine was perpetually greater than the interval time, eventually the wait_time became such that it was always below zero. Once this occurs, application callbacks would cease entirely. To avoid this, run the callback condition in a loop for periodic timers only (not for one-shot config).
jphickey
approved these changes
Nov 4, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that this builds and runs as expected. Tested on my local linux machine and on the RTEMS test platform (QEMU).
Log of timer-test.exe execution for the record captured at: rtems-timer-test-ic-20191030.log
CCB 20191106 - approved for merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the contribution
Fixes #230, fixes #266, fixes #269
Fixes #271, fixes #272, fixes #273, fixes #274
Testing performed
Steps taken to test the contribution:
Built without warnings, all tests passed except osal_timer_UT (nominal result on linux)
executed cfe, successful startup with no warnings
Expected behavior changes
Coverage test updates (more coverage and works for vxworks)
Timer bug fixes
sem-speed-test functional test fix for vxworks
System(s) tested on:
Additional context
None
Contributor Info
Jacob Hageman - NASA/GSFC